!!! TASK: Split dimensionspacepoints into separate table to reduce data duplication #4790

kitsunet · 2023-12-01T17:28:29Z

This should reduce stored data amounts, also make queries and workspace copies faster.
The hierarchy relations are now entirely limited size field types and should not
need external lookup like "text" fields do.

Note that this needs a ./flow cr:setup and ./flow cr:projectionreplay --projection contentGraph

This should reduce stored data amounts, also make queries and workspace copies faster. The hierarchy relations are now entirely limited size field types and should not need external lookup like "text" fields do.

bwaidelich

Looks good (I was just skimming over it on my phone), thanks for taking care!
Apart from the inline comments I wonder:
We assume, that we have a performance issue and that it is solved with this PR. I have no reason to doubt this, but did you measure that by any chance?

Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Repository/DimensionSpacePoints.php

Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Repository/ContentGraph.php

Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Repository/DimensionSpacePoints.php

kitsunet · 2024-03-13T10:45:07Z

Will take care of the rest, thank you!

But here is one example outcome on my local system, 3 times replay contentGraph on 9.0 and then again 3 times on this branch (with cr:setup before ofc)

That seems like a nice improvement for what is essentially the demo site with a few edits. Data size I cannot verify yet, but it's tricky because what is shown in clients and information_schema is not necessarily the full picture. Currently trying to get sensible numbers.

bwaidelich

Looks great already, just one more question/comment.

But here is one example outcome on my local system

Impressive, almost 30% speed improvement \o/
@kitsunet thanks for going the extra mile and providing a basic benchmark.

Data size I cannot verify yet

I don't think, that's that important really.. it will most probably be in the same region

Neos.ContentGraph.DoctrineDbalAdapter/src/DoctrineDbalContentGraphProjection.php

Neos.ContentGraph.DoctrineDbalAdapter/src/DoctrineDbalContentGraphSchemaBuilder.php

dlubitz · 2024-03-14T09:47:04Z

...tentGraph.DoctrineDbalAdapter/src/Domain/Projection/ProjectionIntegrityViolationDetector.php

@@ -67,8 +67,8 @@ public function hierarchyIntegrityIsProvided(): Result
        }

        $invalidlyHashedHierarchyRelationRecords = $this->client->getConnection()->executeQuery(
-            'SELECT * FROM ' . $this->tableNamePrefix . '_hierarchyrelation
-                WHERE dimensionspacepointhash != MD5(dimensionspacepoint)'
+            'SELECT * FROM ' . $this->tableNamePrefix . '_hierarchyrelation h LEFT JOIN ' . $this->tableNamePrefix . '_dimensionspacepoints dsp ON dsp.hash = h.dimensionspacepointhash


I don't know what the purpose for this check is. But after it's not fully the same as before. So we just check if the entry for a given hash is present in _dimensionspacepoints. But before we also checked if the md5-hashed value is equal to the hash.

Mmmm, we can't know if the hash is the right one though, either it exists in the DSP table or it's invalid, if it exists in there IMHO we can assume it matches the dimension coordinates. We could add a separate check that reads all DSP entries and compares hash and getting the hash from the coordinates in the same row if we really want to.

I'm fine with keeping it as it is. Just wanted to point out this difference.

...tentGraph.DoctrineDbalAdapter/src/Domain/Projection/ProjectionIntegrityViolationDetector.php

dlubitz

Looks good to me by reading!

mhsdesign · 2024-03-15T14:07:42Z

Bastians pr should go first ;)

Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Repository/DimensionSpacePoints.php

…acepoints

mhsdesign

Wow thank you ^^
Interestingly the speed gain is enormous with my 270 events of the neos demo + some extra stuff

Command	Before (average of 3)	After (average of 3)
`time ./flow cr:projectionReplayAll --force`	~21s	~1.5s
`time ./flow cr:projectionReplay contentGraph --force`	~29s	~5s

(the reason why there is a difference between all and one is some odd db lock, and also somehow this change avoids running into the lock state for a projectionReplayAll (previously the db had 95% cpu intensity))

mhsdesign · 2024-03-18T09:55:59Z

Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Repository/DimensionSpacePointsRepository.php

+        $coordinates = $this->getCoordinatesByHashFromRuntimeCache($hash);
+        if ($coordinates === null) {
+            $this->fillRuntimeCacheFromDatabase();
+            $coordinates = $this->getCoordinatesByHashFromRuntimeCache($hash);
+        }


ha this is a bit hacky as we dont pass this repository as singleton that means it cannot share its runtime cache and would otherwise not pick up on new OriginDimensionSpacePoints, but i consider this a very safe runtime cache as nothing can go wrong (despite it being outdated, in which case we refetch)?

true, we could even make this static if we wanted LOL

nah its fine thx, i guess asking the db like this is good enough and it will also work for async / parallel

Neos.ContentGraph.DoctrineDbalAdapter/src/DoctrineDbalContentGraphProjectionFactory.php

kitsunet · 2024-03-18T19:10:12Z

The reading side now shares one instance and the writing side uses the repository transactional with the respective other entry written (NodeRecord / HierachyRelation)

mhsdesign · 2024-03-18T19:48:27Z

Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Projection/NodeRecord.php

+            $tableNamePrefix,
+            $nodeAggregateId,
+            $originDimensionSpacePoint,
+            $originDimensionSpacePointHash,
+            $properties,
+            $nodeTypeName,
+            $classification,
+            $timestamps


mhsdesign

Thanks for the improvement, its a nice bike.

mhsdesign · 2024-03-19T07:15:03Z

A just a noob question @kitsunet

Why don’t we join but handle that separately? I guess performance and simplicity?

…onspacepoints" This reverts commit c3a81af, reversing changes made to 117c052.

mhsdesign · 2024-04-24T12:29:52Z

Neos.ContentGraph.DoctrineDbalAdapter/src/DoctrineDbalContentGraphSchemaBuilder.php

        return $table
-            ->addIndex(['childnodeanchor'])
-            ->addIndex(['contentstreamid'])
-            ->addIndex(['parentnodeanchor'])
-            ->addIndex(['contentstreamid', 'childnodeanchor', 'dimensionspacepointhash'])
+            ->setPrimaryKey(['childnodeanchor', 'contentstreamid', 'dimensionspacepointhash', 'parentnodeanchor'])
            ->addIndex(['contentstreamid', 'dimensionspacepointhash']);


lol after debugging i found out that this change here slows down the replay of ContentStreamWasForked events dramatically

I can confirm, this fixes replay for me:

I would suggest that we revert that part for now as a "hotfix". Afterwards, we could investigate why that kills the database

…acepoints" This reverts commit c3a81af, reversing changes made to 117c052.

github-actions bot added the 9.0 label Dec 1, 2023

kitsunet force-pushed the task/split-dimensionspacepoints branch from 038f0df to 4274928 Compare March 12, 2024 22:50

kitsunet marked this pull request as ready for review March 12, 2024 22:51

kitsunet force-pushed the task/split-dimensionspacepoints branch 2 times, most recently from 2239ab0 to 6bef019 Compare March 12, 2024 23:06

TASK: Normalize DimensionSpacePoints to separate table

3cf14ed

This should reduce stored data amounts, also make queries and workspace copies faster. The hierarchy relations are now entirely limited size field types and should not need external lookup like "text" fields do.

kitsunet force-pushed the task/split-dimensionspacepoints branch from 6bef019 to 3cf14ed Compare March 12, 2024 23:16

kitsunet added 3 commits March 13, 2024 00:43

Fix behat test

cdff3d3

Fix ProjectionIntegrityViolationDetector

074f319

Make stan happier

266ecca

bwaidelich reviewed Mar 13, 2024

View reviewed changes

mhsdesign mentioned this pull request Mar 13, 2024

9.0 Dev & Debugging add human readable node database view #4937

Open

kitsunet added 2 commits March 13, 2024 14:27

Some code cleanup

d468dd3

Make stan happy

0c18558

bwaidelich reviewed Mar 14, 2024

View reviewed changes

Neos.ContentGraph.DoctrineDbalAdapter/src/DoctrineDbalContentGraphProjection.php Outdated Show resolved Hide resolved

dlubitz reviewed Mar 14, 2024

View reviewed changes

Neos.ContentGraph.DoctrineDbalAdapter/src/DoctrineDbalContentGraphSchemaBuilder.php Show resolved Hide resolved

dlubitz reviewed Mar 14, 2024

View reviewed changes

DimensionSpacePoints should be created in transaction

71504a5

dlubitz reviewed Mar 14, 2024

View reviewed changes

...tentGraph.DoctrineDbalAdapter/src/Domain/Projection/ProjectionIntegrityViolationDetector.php Show resolved Hide resolved

dlubitz approved these changes Mar 14, 2024

View reviewed changes

mhsdesign marked this pull request as draft March 15, 2024 14:07

mhsdesign reviewed Mar 15, 2024

View reviewed changes

Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Repository/DimensionSpacePoints.php Outdated Show resolved Hide resolved

kitsunet added 2 commits March 15, 2024 18:42

Rename to DimensionSpacePointsRepository

42235cc

Merge branch '9.0' into task/split-dimensionspacepoints

5f477cc

kitsunet marked this pull request as ready for review March 15, 2024 17:44

kitsunet added 2 commits March 15, 2024 18:56

Merge remote-tracking branch 'origin/9.0' into task/split-dimensionsp…

4f9273d

…acepoints

Merge remote-tracking branch 'origin/9.0' into task/split-dimensionsp…

94cbe4f

…acepoints

mhsdesign reviewed Mar 18, 2024

View reviewed changes

Improved usage of DimensionSpacePointsRepository

882b572

mhsdesign reviewed Mar 18, 2024

View reviewed changes

mhsdesign approved these changes Mar 18, 2024

View reviewed changes

mhsdesign mentioned this pull request Mar 18, 2024

FEATURE: Introduce NodeIdentity #4868

Closed

35 tasks

kitsunet merged commit c3a81af into neos:9.0 Mar 18, 2024
9 checks passed

mhsdesign changed the title ~~Split dimensionspacepoints into separate table to reduce data duplication~~ !!! TASK: Split dimensionspacepoints into separate table to reduce data duplication Mar 30, 2024

mhsdesign added a commit to mhsdesign/neos-development-collection that referenced this pull request Apr 24, 2024

Revert "Merge pull request neos#4790 from kitsunet/task/split-dimensi…

399947e

…onspacepoints" This reverts commit c3a81af, reversing changes made to 117c052.

mhsdesign mentioned this pull request Apr 24, 2024

!!! BUGFIX: Fix schema of hierarchy relation table #5009

Merged

mhsdesign reviewed Apr 24, 2024

View reviewed changes

bwaidelich added a commit that referenced this pull request Apr 24, 2024

Revert "Merge pull request #4790 from kitsunet/task/split-dimensionsp…

6c3cff2

…acepoints" This reverts commit c3a81af, reversing changes made to 117c052.

mhsdesign mentioned this pull request Apr 24, 2024

4790 Followup, rethink the optimisations #5011

Open

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

!!! TASK: Split dimensionspacepoints into separate table to reduce data duplication #4790

!!! TASK: Split dimensionspacepoints into separate table to reduce data duplication #4790

kitsunet commented Dec 1, 2023 •

edited

Loading

bwaidelich left a comment

kitsunet commented Mar 13, 2024

bwaidelich left a comment

dlubitz Mar 14, 2024

kitsunet Mar 14, 2024 •

edited

Loading

dlubitz Mar 14, 2024

dlubitz left a comment

mhsdesign commented Mar 15, 2024

mhsdesign left a comment

mhsdesign Mar 18, 2024

kitsunet Mar 18, 2024

mhsdesign Mar 18, 2024

kitsunet commented Mar 18, 2024

mhsdesign Mar 18, 2024

mhsdesign left a comment

mhsdesign commented Mar 19, 2024

mhsdesign Apr 24, 2024

bwaidelich Apr 24, 2024

mhsdesign Apr 24, 2024

!!! TASK: Split dimensionspacepoints into separate table to reduce data duplication #4790

!!! TASK: Split dimensionspacepoints into separate table to reduce data duplication #4790

Conversation

kitsunet commented Dec 1, 2023 • edited Loading

bwaidelich left a comment

Choose a reason for hiding this comment

kitsunet commented Mar 13, 2024

bwaidelich left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kitsunet Mar 14, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dlubitz left a comment

Choose a reason for hiding this comment

mhsdesign commented Mar 15, 2024

mhsdesign left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kitsunet commented Mar 18, 2024

Choose a reason for hiding this comment

mhsdesign left a comment

Choose a reason for hiding this comment

mhsdesign commented Mar 19, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kitsunet commented Dec 1, 2023 •

edited

Loading

kitsunet Mar 14, 2024 •

edited

Loading